Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: compiler invocations #12211

Merged
merged 7 commits into from
May 31, 2023

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

Found some places needing minor refactors during reviewing #12205

How should we test and review this PR?

Commit by commit. Each has its own explanation.

The first four shouldn't be controversial. However, we need people to scrutinize the last two. All static rustflags additions were moved into a single functions. This shouldn't have any behavior change.

Additional information

After this, I'll do a follow-up to extract flags building process. It is expected to make it clearer and safer for building static and dynamic flags.

@rustbot
Copy link
Collaborator

rustbot commented May 31, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2023
Apparently it was a bit confusing where `rustflags` is from
when reading this piece of code.
@weihanglo weihanglo force-pushed the refactor-compiler-invocation branch from d21e8a8 to 5256dec Compare May 31, 2023 18:16
@weihanglo weihanglo force-pushed the refactor-compiler-invocation branch from 5256dec to fb92ee5 Compare May 31, 2023 19:01
@@ -100,7 +100,7 @@ fn profile_selection_build() {
[FINISHED] dev [unoptimized + debuginfo] [..]
"
)
.with_stdout_does_not_contain("[..] -C debuginfo=0[..]")
.with_stderr_does_not_contain("[..] -C debuginfo=0[..]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these tests were testing the wrong thing, should that be its own commit?

This is also why I don't like testing for something not existing...

@@ -1334,13 +1334,13 @@ fn cargo_default_env_metadata_env_var() {
[COMPILING] bar v0.0.1 ([CWD]/bar)
[RUNNING] `rustc --crate-name bar bar/src/lib.rs [..]--crate-type dylib \
--emit=[..]link \
-C prefer-dynamic[..]-C debuginfo=2 \
-C prefer-dynamic[..]-C debuginfo=2 [..]\

This comment was marked as resolved.

It was unnecessary to pass `spilt-debuginfo` if there is no debuginfo.
Tests are touched here only for matching rustflags invocation stderr
in the original test suite.
This is for linker arguments from build scripts, so should live there.
Make it clear by separating static and dynamic rustflags.
Make it clear by separating static and dynamic rustdocflags.
@weihanglo weihanglo force-pushed the refactor-compiler-invocation branch from fb92ee5 to d4067e4 Compare May 31, 2023 19:46
@epage
Copy link
Contributor

epage commented May 31, 2023

@bors r+

@bors
Copy link
Contributor

bors commented May 31, 2023

📌 Commit d4067e4 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2023
@bors
Copy link
Contributor

bors commented May 31, 2023

⌛ Testing commit d4067e4 with merge e755955...

@bors
Copy link
Contributor

bors commented May 31, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing e755955 to master...

@bors bors merged commit e755955 into rust-lang:master May 31, 2023
@weihanglo weihanglo deleted the refactor-compiler-invocation branch May 31, 2023 20:42
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2023
Update cargo

14 commits in f7b95e31642e09c2b6eabb18ed75007dda6677a0..b0fa79679e717cd077b7fc0fa4166f47107f1ba9
2023-05-30 19:25:02 +0000 to 2023-06-03 14:19:48 +0000
- Emit error when users try to use a toolchain via the `add` or `install` command (rust-lang/cargo#12226)
- Support "default" option for `build.jobs` (rust-lang/cargo#12222)
- Fix typo in changelog (rust-lang/cargo#12227)
- chore: Sort `-Z` flags match statement (rust-lang/cargo#12223)
- Update curl-sys (rust-lang/cargo#12218)
- Bump to 0.73.0; update changelog (rust-lang/cargo#12219)
- refactor: housekeeping for 1.70.0 (rust-lang/cargo#12217)
- nit: fix typo in changelog for 1.70 (rust-lang/cargo#12215)
- Remove a noop `.clone` (rust-lang/cargo#12213)
- refactor: compiler invocations (rust-lang/cargo#12211)
- cargo clean: use `remove_dir_all` (rust-lang/cargo#11442)
- Add a small note about indexes ignoring SemVer build metadata. (rust-lang/cargo#12206)
- Revert "chore: detect the channel a PR wants to merge into" (rust-lang/cargo#12204)
- Don't distinguish `Debuginfo::None` and `Debuginfo::Explicit(None)` (rust-lang/cargo#12205)

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 11, 2023
Update cargo

14 commits in f7b95e31642e09c2b6eabb18ed75007dda6677a0..b0fa79679e717cd077b7fc0fa4166f47107f1ba9
2023-05-30 19:25:02 +0000 to 2023-06-03 14:19:48 +0000
- Emit error when users try to use a toolchain via the `add` or `install` command (rust-lang/cargo#12226)
- Support "default" option for `build.jobs` (rust-lang/cargo#12222)
- Fix typo in changelog (rust-lang/cargo#12227)
- chore: Sort `-Z` flags match statement (rust-lang/cargo#12223)
- Update curl-sys (rust-lang/cargo#12218)
- Bump to 0.73.0; update changelog (rust-lang/cargo#12219)
- refactor: housekeeping for 1.70.0 (rust-lang/cargo#12217)
- nit: fix typo in changelog for 1.70 (rust-lang/cargo#12215)
- Remove a noop `.clone` (rust-lang/cargo#12213)
- refactor: compiler invocations (rust-lang/cargo#12211)
- cargo clean: use `remove_dir_all` (rust-lang/cargo#11442)
- Add a small note about indexes ignoring SemVer build metadata. (rust-lang/cargo#12206)
- Revert "chore: detect the channel a PR wants to merge into" (rust-lang/cargo#12204)
- Don't distinguish `Debuginfo::None` and `Debuginfo::Explicit(None)` (rust-lang/cargo#12205)

r? `@ghost`
@ehuss ehuss added this to the 1.72.0 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants